Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add message definitions for the parameter component #241

Merged
merged 12 commits into from
Aug 17, 2022

Conversation

ivanpauno
Copy link
Contributor

🎉 New feature

Summary

Add message definitions to be used by the ignition transport parameter component.
See gazebosim/gz-transport#305.

Test it

See gazebosim/gz-sim#1431 for details of a demo.
Replaces #211.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

@ivanpauno ivanpauno requested a review from caguero as a code owner April 8, 2022 18:21
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Apr 8, 2022
@codecov
Copy link

codecov bot commented Apr 8, 2022

Codecov Report

Merging #241 (c1510f7) into ign-msgs8 (8e10c8d) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           ign-msgs8     #241   +/-   ##
==========================================
  Coverage      85.38%   85.38%           
==========================================
  Files              9        9           
  Lines            951      951           
==========================================
  Hits             812      812           
  Misses           139      139           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e10c8d...c1510f7. Read the comment docs.

Copy link
Contributor

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that Any makes more sense if everything is going to stay protobuf.

proto/ignition/msgs/parameter.proto Outdated Show resolved Hide resolved
proto/ignition/msgs/parameter_name.proto Outdated Show resolved Hide resolved
proto/ignition/msgs/parameter_value.proto Outdated Show resolved Hide resolved
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
* Update copyright year 2021->2022.

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno ivanpauno force-pushed the ivanpauno/parameters-component8 branch from c1510f7 to 933c1e9 Compare May 24, 2022 19:58
@codecov
Copy link

codecov bot commented May 24, 2022

Codecov Report

Merging #241 (69fde66) into ign-msgs8 (065a3ce) will not change coverage.
The diff coverage is n/a.

❗ Current head 69fde66 differs from pull request most recent head abc8ed6. Consider uploading reports for the commit abc8ed6 to get more accurate results

@@            Coverage Diff             @@
##           ign-msgs8     #241   +/-   ##
==========================================
  Coverage      96.64%   96.64%           
==========================================
  Files              9        9           
  Lines            953      953           
==========================================
  Hits             921      921           
  Misses            32       32           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

caguero and others added 4 commits June 28, 2022 02:24
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno ivanpauno requested a review from mjcarroll July 4, 2022 19:22
@chapulina chapulina added the enhancement New feature or request label Jul 23, 2022
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno ivanpauno requested a review from caguero August 3, 2022 18:54
Copy link
Collaborator

@caguero caguero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Just a minor comment but I'm approving.

proto/ignition/msgs/parameter_declaration.proto Outdated Show resolved Hide resolved
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno ivanpauno requested a review from caguero August 4, 2022 18:42
@ivanpauno
Copy link
Contributor Author

@caguero friendly ping

@scpeters scpeters merged commit 1e1cbe8 into ign-msgs8 Aug 17, 2022
@scpeters scpeters deleted the ivanpauno/parameters-component8 branch August 17, 2022 23:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request 🏯 fortress Ignition Fortress
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants